Inherit much of geojson parser from GMT #8
Conversation
| @@ -1,7 +1,7 @@ | |||
| import unittest | |||
| from pathlib import Path | |||
|
|
|||
| from urbanopt_des.urbanopt_geojson import URBANoptGeoJSON | |||
| from urbanopt_des.urbanopt_geojson import DESGeoJSON | |||
There was a problem hiding this comment.
I renamed the uo-des geojson parsing class, to not conflict with the one from the GMT. Should it be different?Happy to change the name to something else if that makes more sense.
There was a problem hiding this comment.
I think we should move all the methods out of DESGeoJSON... looks like the only one remaining is create_aggregated_representation. Can we move that over to GMT's GeoJSON?
urbanopt_des/urbanopt_geojson.py
Outdated
There was a problem hiding this comment.
Should we change this filename too, as well as the class name?
| @@ -1,7 +1,7 @@ | |||
| import unittest | |||
| from pathlib import Path | |||
|
|
|||
| from urbanopt_des.urbanopt_geojson import URBANoptGeoJSON | |||
| from urbanopt_des.urbanopt_geojson import DESGeoJSON | |||
There was a problem hiding this comment.
I think we should move all the methods out of DESGeoJSON... looks like the only one remaining is create_aggregated_representation. Can we move that over to GMT's GeoJSON?
Yes, we can easily move that to the GMT. I didn't do so because that method depends on geopandas, and therefore gdal (which has about a zillion dependencies itself). I was trying to keep that out of the GMT. Please opine :) |
|
oh, then we shouldn't move. However, we should probably recreate without geopandas. Shouldn't be hard, but let's write tests first. Does GMT rely on pandas at all? |
Yes, regular pandas is used in the GMT. I've idly thought of replacing that with Polars, or of trying to refactor to just use stdlib, but not very seriously (also, Teams just blew up on me, forcing a reinstall, not just a restart, lol). |
urbanopt/geojson-modelica-translator#694 added most of the geojson parsing functionality from this repo into the GMT. Now we depend on the GMT, inherit from that class, and extend with geopandas only in this repo.
Also updated ruff, which then helped me clean up type hints. We can use the modern style because we require at least Python 3.10.